-
Notifications
You must be signed in to change notification settings - Fork 663
[rush-sdk]: add named export support for CommonJS compatibility #5539
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
common/changes/@microsoft/rush/chore-optimize-named-exports_2026-01-06-12-36.json
Outdated
Show resolved
Hide resolved
| import * as path from 'node:path'; | ||
|
|
||
| import { FileSystem, Import, Path } from '@rushstack/node-core-library'; | ||
| import { initSync, parse } from 'cjs-module-lexer'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We're already generating types with named exports and ESM modules, so we shouldn't need to try to infer the names from the CJS.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does your suggestion involve reading .d.ts files to confirm what named exports are available? Which of the following approaches do you think is better:
- ast-grep
- TypeScript
- RegExp
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Take a look at https://www.npmjs.com/package/es-module-lexer and analyzing the ESM modules we already generate in rush-lib.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The reason for using [email protected] here is that Node.js also uses this version, ensuring that the export identifiers we add can remain consistent with the actual Node.js runtime (equivalent to letting Node.js run first to see exactly which export names it can recognize). If different lexer implementations or lexer-executed source code are used, there might be some risk of inconsistency with the actual runtime, potentially leading to runtime issues. However, the risk seems acceptable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some updates.
After discussed with @octogonz , he pointed out that I can use WebpackDeepImportsPlugin to generate the name exports code for cjs artifacts.
So, I have done some refactoring. Here are the changes:
- Add
ExportsInfoinWebpackDeepImportsPluginand use it to generate named exports code. This change will impactrush-lib/lib/**/*.js. - Use
rush-lib/lib/**/*.jsfile as the source to generate named exports code forrush-sdk/lib.
Other unrelated changes:
- Fix stdout path style for windows https://github.com/microsoft/rushstack/pull/5539/files#diff-21c8cbc0b3de7148028acc7fc1fa41060f7b18531f3354d6eaa153da8a6d3f83R141
This commit enhances @rushstack/rush-sdk to support named imports when the package is consumed via ESM project.
…26-01-06-12-36.json Co-authored-by: Ian Clanton-Thuon <[email protected]>
fc42dde to
7c42835
Compare
Summary
This commit enhances @rushstack/rush-sdk to support named imports when the package is consumed via ESM project.
Resolve #5506
Details
Here's the changes.
libraries/rush-sdk/package.json
[email protected]as devDependency. Required for extracting named exports during stub generation. Version2.1.0is consistent with the built-in version in Node.js.libraries/rush-sdk/config/jest.config.json
libraries/rush-sdk/src/generate-stubs.ts
extractNamedExports(source)function to parse CommonJS modules and extract export names.libraries/rush-sdk/src/test/snapshots/script.test.ts.snap
libraries/rush-sdk/src/test/script.test.ts
libraries/rush-sdk/src/test/build-assets-with-named-exports.test.ts (NEW)
lib-shim/andlib/assets.libraries/rush-sdk/webpack.config.js
webpack.BannerPluginimport from webpack. Reads all export specifiers from@microsoft/rush-liband inject the dynamic exports placeholder code by using thsBannerPlugin.The core improvement enables consumers to use both default imports and named destructuring:
How it was tested
Unit tests were added.
Impacted documentation
None